Skip to content

DNM Demonstrate segfault with variable-width strings #38

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

DNM Demonstrate segfault with variable-width strings #38

wants to merge 2 commits into from

Conversation

@crusaderky
Copy link
Contributor

@crusaderky crusaderky commented Oct 17, 2025 •
edited
Loading

@crusaderky crusaderky mentioned this pull request Oct 17, 2025
Open
@crusaderky
Copy link
Contributor Author

Unsure why CI is not being triggered?

@FrancescAlted
Copy link
Member

Unsure why CI is not being triggered?

Because there is not a proper CI set yet (we typically test this plugin manually with PyTables). Would you like to provide one?

Also, I don't see the fix in the PR (apparently it was reverted in a commit?). Also missing the PR for the hdf5-blosc2 plugin.

@crusaderky
Copy link
Contributor Author

Unsure why CI is not being triggered?

Because there is not a proper CI set yet (we typically test this plugin manually with PyTables). Would you like to provide one?

Yes, happy to work on it.

Also, I don't see the fix in the PR (apparently it was reverted in a commit?).

Yes, I'm working on it.
I just got to a roadblock. I discovered that the underlying issue is that, for H5T_VARIABLE dtypes, libhdf5 invokes neither the can_apply nor the set_local callbacks. As a result cd_values[0:5] are uninitialised (or initialised to 0, if you are using h5py). It can be worked around by forcing typesize=1 when writing, but you'll end up with uninitialised values on disk which is really not ideal. I need to investigate libhdf5 to see if I and h5py are doing anything wrong or if we need an libhdf5 fix.

Also missing the PR for the hdf5-blosc2 plugin.

Also working on it. It's the same problem.

@crusaderky crusaderky force-pushed the strings branch 8 times, most recently from 83c52f8 to f537f9f Compare October 22, 2025 13:11
crusaderky
crusaderky commented Oct 22, 2025
View reviewed changes
src/blosc_filter.c
const char* complist;
char errmsg[256];

assert(cd_nelmts >= 4);
Copy link
Contributor Author

@crusaderky crusaderky Oct 22, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test_strings crashes here if you remove the workaround hack from it. blosc_set_local did not run so this is valued 0.

@crusaderky crusaderky force-pushed the strings branch 3 times, most recently from a906df9 to 57c53b2 Compare October 22, 2025 17:17
@crusaderky crusaderky closed this Oct 23, 2025
@crusaderky crusaderky reopened this Oct 23, 2025
@crusaderky crusaderky mentioned this pull request Oct 24, 2025
Open
@crusaderky
Copy link
Contributor Author

crusaderky commented Oct 24, 2025 •
edited
Loading

Opened upstream issue: HDFGroup/hdf5#5942
This PR has been rebased on top of #42.

@crusaderky crusaderky mentioned this pull request Oct 24, 2025
Open
@crusaderky crusaderky changed the title WIP Segfault with variable-width strings DNM Demonstrate segfault with variable-width strings Oct 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@crusaderky @FrancescAlted